I'm thinking of:

1. putting this in euler.h since it seems like it will fit better there
2. using a parameter to control what order the euler rotations will take place in the quaternion
3. If the xyz combinations have to happen in the name, using a macro function to create all tests for all combinations of xyz
4. I'll also add glm_euler_by_order + glm_mat4_quat in tests as another test.

I'd like some feedback of these changes

@telephone001 many thanks for the work,

just a reminder, as mentioned: #306 (comment) better to double check this before merge

docs/euler.rst:

Rotation Conveniention
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Current *cglm*'s euler functions uses these convention:

* Tait–Bryan angles (x-y-z convention)
* Intrinsic rotations (pitch, yaw and roll).
  This is reserve order of extrinsic (elevation, heading and bank) rotation
* Right hand rule (actually all rotations in *cglm* use **RH**)
* All angles used in *cglm* are **RADIANS** not degrees

In the future euler api can be improved even may provide global/extrinsic too but currently as docs states we must stick above constraints.

Currently my functions work like doing glm_quat_mul 3 times with the axis vectors. But when compared to the glm_euler_by_order, all the values are negative compared to using glm_euler_by_order.

I pushed just to give an update. This commit is messy because I'm in the process of debugging it and debugging tests in euler.c is harder than in quat.h.

Currently i'm not really getting what extrinsic and intrinsic angles are. I based my calculations thinking that xyz corresponds to yaw pitch roll.

Currently i'm not really getting what extrinsic and intrinsic angles are.

https://dominicplein.medium.com/extrinsic-intrinsic-rotation-do-i-multiply-from-right-or-left-357c38c1abfd explains and shows extrinsic and intrinsic graphically (animated) which could help to understand better.

Tait-Bryan vs. Classic,
Intrinsic vs. Extrinsic rotations...:

https://en.wikipedia.org/wiki/Euler_angles
https://danceswithcode.net/engineeringnotes/rotations_in_3d/rotations_in_3d_part1.html

I'll check glm_euler_by_order to validate glm_euler_by_order (..., GLM_EULER_XYZ, ..) is same as glm_euler_xyz()... asap

Currently all my tests pass when only checking multiplication of quaternion axises. I'm pretty sure glm_euler_by_order is the same as glm_euler_xyz and its variants because it returns the same results when testing. I'm thinking the error might be in the mat4 ->quat or euler->mat4.

Also, I remember not being able to make different tester functions even when I declared and entered them. I did the exact same thing I did when making the tests in testquat.h. they worked in testquat but not euler.c. Could I possibly be missing something?

@telephone001 many thanks, I'll try to take a deeper look asap

I'm thinking the error might be in the mat4->quat or euler->mat4.

Based on those action logs, I suspect you should be testing x == q || x == -q (e.g., test_assert_quat_eq_abs)

when test_assert_quat_eq_abs used, all tests pass. But I still want the directions of my function to be right and consistent with the other glm functions

That may be a bit difficult given the nature of glm_mat4_quat.

Another question is whether the ordering of parameters be flipped? Out pointers (i.e., "dest") parameters are usually the last parameters to a function, e.g., glm_euler_by_order.

Oh yeah I'll make the destination the last parameter if thats needed. Some of the quat functions have the output as the first parameter though when they are creating a new quaternion.

I'm thinking of either removing the euler->mat4->quat tests and merging then dealing with mat4 in another request or making another pull request and fixing mat4 there then after its done, go back here and merge.

I also figured out how to make the tests separate. I'm gonna be busy for a while but later I'm gonna try and make better tests for euler_xyz and possibly fix them

@telephone001 thanks for the work. I'll also check tests.

May your euler->quat and mat4->quat generate different rotations? e.g. short way <-> long way rotation? We should add clarification in docs where it is needed e.g mat4_quat whether it is short or long, TODOs. In this case comparing like:

/* verify that it acts the same as glm_euler_by_order */
mat4 expected_mat4;
glm_euler_by_order(angles, GLM_EULER_XZY, expected_mat4);
glm_mat4_quat(expected_mat4, expected);

ASSERTIFY(test_assert_quat_eq(result, expected));

may not be valid approach, rotating a vector then comparing rotated vectors may be an option. But it would be nice to get similar quaternion by euler->quat and mat4->quat.

@recp
Oh I forgot that could happen. Euler_xyz might be fine. I tried applying the mat4 and quaternion to the same vec3 and they both returned the same result. I'm pretty sure mine computes the longer path if angle > 180* because it always gets the same results as rotating by 3 axis quaternions with the angles just plugged in. I'll try and fix it soon

@telephone001 thanks 👍

I cant validate that mat4_quat always gives short path, I'll check it when I available asap.

@recp
I'm thinking that instead of forcing the quaternion to always be the shortest path always, we can just change the quaternion to the short path when needed in interpolation. I think the path it takes only matters in interpolation because if you just want to apply a quaternion to a position, you just use the hamiltonian product and the position gets oriented instantly so I'm gonna take gottfriedleibniz's advice and just use the abs testers

@telephone001, are we ready to merge this PR? I'll check the implementation this weekend.

One question:

since cglm supports LH operations too, I guess we may need to provide LH versions too e.g. glm_euler_xyz_quat_lh().

One option is to implement this as RH then LH asap to prevent this become giant PR

see: #322

@recp
Oh yeah I thought you were reviewing my pr last week. Sorry I couldn't respond sooner.
I think they might already be in right hand. Do I need to rename them to glm_euler_xyz_quat_rh()?
I'll get it in left hand in another PR.

Oh yeah I thought you were reviewing my pr last week

Sorry for the delay :(

Do I need to rename them to glm_euler_xyz_quat_rh()?

That would be awesome. I think we can move LH/RH related implementations into handed folder similar to clipspace and wrap them by options e.g. #ifdef CGLM_FORCE_LEFT_HANDED. See cam.h which can help to implement LH, RH skeleton ( for instance files in separate handed folder and wrapper in euler.h ).

If you dont enough time I can merge this PR and add _rh() suffixes and move them to handed folder ( if there is a better name we can continue with it )

I'll get it in left hand in another PR.

👍

Should I make euler_to_quat_rh.h and euler_to_quat_lh.h in that folder?

Also instead of clipspace and handed folders, I just have 1 handed folder and put all handed stuff in that folder?

currently I have a handed folder and also the rh stuff in that folder. I didn't know what to name the macro that controls righthandedness so I just used CGLM_FORCE_LEFT_HANDED

Should I make euler_to_quat_rh.h and euler_to_quat_lh.h in that folder?

euler_rh.h/euler_lh.h would be enough but euler_to_quat_rh.h is also ok.

Also instead of clipspace and handed folders, I just have 1 handed folder and put all handed stuff in that folder?

clipspace keeps stuff related clipspace and/or lh/rh... I think handed can just keep stuff only related to lh/rh.

using CGLM_FORCE_LEFT_HANDED is right path to go for now 👍

@telephone001 the PR is merged, many thanks for your contributions 🚀

Let's continue with separate PR for LH versions and keep track bugs and improvements for a while :)